Skip to content

feat: move logs agents to its own tile #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 33 commits into from
May 7, 2025
Merged

feat: move logs agents to its own tile #2

merged 33 commits into from
May 7, 2025

Conversation

@jor2
Copy link
Member

@jor2 jor2 commented Mar 13, 2025

Description

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@jor2 jor2 self-assigned this Mar 13, 2025
@jor2 jor2 changed the title feat: Move Observability agents to its own tile feat: Move logs agents to its own tile Mar 18, 2025
ocofaigh
ocofaigh requested changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments, but maybe this is still WIP as I notice the DA inputs still have cloud monitoring inputs there?

@jor2 jor2 marked this pull request as draft March 19, 2025 02:28
@jor2 jor2 marked this pull request as ready for review March 19, 2025 14:12
@jor2 jor2 force-pushed the agent-da branch 2 times, most recently from a43364d to 9a924ef Compare March 19, 2025 16:46
@ocofaigh
Copy link
Contributor

@jor2 Can you add same codeowners as https://github.com/terraform-ibm-modules/terraform-ibm-observability-agents/blob/main/.github/CODEOWNERS

@jor2 jor2 force-pushed the agent-da branch 2 times, most recently from 62b2320 to c93af1d Compare March 20, 2025 19:38
@jor2
Copy link
Member Author

jor2 commented Mar 20, 2025

/run pipeline

1 similar comment
@jor2
Copy link
Member Author

jor2 commented Mar 21, 2025

/run pipeline

@jor2 jor2 requested review from Aashiq-J, ocofaigh and shemau and removed request for daniel-butler-irl March 21, 2025 16:11
@jor2 jor2 changed the title feat: Move logs agents to its own tile feat: move logs agents to its own tile Mar 21, 2025
ocofaigh
ocofaigh requested changes Mar 24, 2025
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • See comments
  • The diagram should show some kind of flow of the agents sending data to a Cloud Logs instance.
  • Can we please use cross variable validation in the DA inputs

README.md Outdated
Use real values instead of "var.<var_name>" or other placeholder values
unless real values don't help users know what to change.
-->
## Usage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change back the readme so it aligns with the module template please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jor2 This is still an open comment

@jor2 jor2 mentioned this pull request Mar 24, 2025
Merged
6 tasks
@ocofaigh
Copy link
Contributor

@jor2 Ca you see if you can add support for terraform-ibm-modules/terraform-ibm-observability-agents#470 in this PR too? I think the logic is slightly incorrect in that PR so you might need to fix it to get it working

@jor2
Copy link
Member Author

jor2 commented Mar 25, 2025

/run pipeline

1 similar comment
@jor2
Copy link
Member Author

jor2 commented Mar 25, 2025

/run pipeline

@ocofaigh ocofaigh mentioned this pull request Mar 26, 2025
Open
6 tasks
ocofaigh
ocofaigh requested changes Mar 26, 2025
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • cloud logs icon is missing from diagram
  • You dont need to commit images/cloud-logs-icon.svg?
  • Can you add this to the renovate.json?
{
  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
  "extends": ["github>terraform-ibm-modules/common-dev-assets:commonRenovateConfig"],
  "customManagers": [
    {
      "customType": "regex",
      "description": "Update agent version to the latest in variables.tf",
      "fileMatch": ["variables.tf$"],
      "datasourceTemplate": "docker",
      "matchStrings": [
        "default\\s*=\\s*\"(?<currentValue>.*)\"\\s*# datasource: (?<depName>[^\\s]+)"
      ]
    }
  ]
}

@jor2
Copy link
Member Author

jor2 commented Apr 1, 2025

/run pipeline

1 similar comment
@jor2
Copy link
Member Author

jor2 commented Apr 1, 2025

/run pipeline

ocofaigh
ocofaigh requested changes Apr 29, 2025
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

@jor2 jor2 requested a review from ocofaigh May 5, 2025 21:11
ocofaigh
ocofaigh requested changes May 6, 2025
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see latest comments - we should be nearly ready to go

ocofaigh
ocofaigh previously approved these changes May 6, 2025
View reviewed changes
@ocofaigh
Copy link
Contributor

ocofaigh commented May 6, 2025

/run pipeline

@jor2
Copy link
Member Author

jor2 commented May 6, 2025

/run pipeline

@jor2
Copy link
Member Author

jor2 commented May 6, 2025

validation passing - https://cloud.ibm.com/devops/pipelines/tekton/3571807b-77d7-411d-a035-017e10cf0d95/runs/3075a77f-6eef-4658-8235-e3f8e0f8f0c6/catalog-onboard/onboard-release?env_id=ibm:yp:us-south

@jor2
Copy link
Member Author

jor2 commented May 7, 2025

/run pipeline

@ocofaigh ocofaigh merged commit e30c28c into main May 7, 2025
2 checks passed
@ocofaigh ocofaigh deleted the agent-da branch May 7, 2025 09:27
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ocofaigh ocofaigh ocofaigh approved these changes

@shemau shemau Awaiting requested review from shemau shemau is a code owner

@Aashiq-J Aashiq-J Awaiting requested review from Aashiq-J Aashiq-J is a code owner

Assignees

@jor2 jor2

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jor2 @ocofaigh @terraform-ibm-modules-ops